-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Workaround bug in EvtFormatMessage #105636
Conversation
Tagging subscribers to this area: @dotnet/area-system-diagnostics-eventlog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use collection expressions? Other than that, looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll poke around a bit to see if I can manage to get a regression test for this without using the customer file but for now just sharing the fix.
Yes that would be great.
Otherwise, this LGTM.
src/libraries/System.Diagnostics.EventLog/src/System/Diagnostics/Reader/NativeWrapper.cs
Show resolved
Hide resolved
src/libraries/System.Diagnostics.EventLog/src/System/Diagnostics/Reader/NativeWrapper.cs
Outdated
Show resolved
Hide resolved
Corrected some mismatched active issues, filed a new active issue for a test crash in linq.expressions. With that BA is passing. |
/backport to release/8.0-staging |
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/10183531351 |
Fix #100198.
This Windows API will still try to write to a buffer if you tell it the buffer is 0 length and the string you're reading happens to be 0 length. We previously avoided an AV here because we were passing a StringBuilder, but that changed with the interop generator refactoring which passed
null
. Avoid the bug that by passing in a 1-character buffer rather than null.I've tested manually using the file provided in the issue repro. I'll poke around a bit to see if I can manage to get a regression test for this without using the customer file but for now just sharing the fix.